Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSX compatibility #4

Merged
merged 1 commit into from
Jul 27, 2022
Merged

OSX compatibility #4

merged 1 commit into from
Jul 27, 2022

Conversation

casperisfine
Copy link

gettid() isn't available on OSX. But we can use the Ruby's thread_native.h to get an identifier.

@ivoanjo
Copy link
Owner

ivoanjo commented Jul 27, 2022

Thanks for this! One thing I spotted is that https://ui.perfetto.dev/ seems not too like the big tids we get on macOS. E.g. I just got this trace for example1.json: https://gist.github.com/ivoanjo/31e4699d6dc74dd3e9c042f910aa989b

and here's how it looks in perfetto (this example has 4 threads):

Screen Shot 2022-07-27 at 10 00 11

But... if I just search and replace on my text editor the big thread ids with smaller ids, it works lol:

Screen Shot 2022-07-27 at 10 02 09

I'll try to report it upstream, but in the meanwhile, any preference on how to work around this? I guess we could % or otherwise truncate bigger values, but it is a bit of a shame that it becomes a number with no connection to anything people see on their Ruby VM.

@casperisfine
Copy link
Author

Hum, that's annoying indeed.

At some point I wanted to provide a small incremental thread_id in the hook arguments, it's especially important because two Ruby threads may have the same native thread id (threads can be re-used in some cases).

I'll make a PR and see if Koichi is open to it.

@casperisfine
Copy link
Author

Upstream PR: ruby/ruby#6189

@casperisfine
Copy link
Author

Hum, I just though that short term this library could use a thread local variable and an atomic to generate that same serial identifier. Let me try.

@casperisfine
Copy link
Author

I discussed the upstream PR with Koichi, it may or may not be acceptable, it depends on another things he's working on, so we'll know in a bit.

In the meantime I'll use a thread local to generate that same serial.

@ivoanjo
Copy link
Owner

ivoanjo commented Jul 27, 2022

Sounds reasonable. I'd still probably keep the task id for linux, since it seems like a more useful value than the not-visible-to-users serial. (?)

`gettid()` isn't available on OSX.

But we can use the Ruby's atomic and _Thread_local store
to generate a serial thread id.
@casperisfine
Copy link
Author

I'd still probably keep the task id for linux, since it seems like a more useful value than the not-visible-to-users serial. (?)

Up to you. I can do that if you prefer. I'd say the advantage of serial id is that it's incremental, so 0 is the main thread, 1 is the first thread you ever created etc etc.

But I can add some code to check if gettid() is present, just let me know.

@ivoanjo
Copy link
Owner

ivoanjo commented Jul 27, 2022

I'm happy with the current version. Can always re-add the thread id later. (Happy to hear feedback from the community here)

Thanks for the contribution! I'll make a release a bit later today :)

@ivoanjo ivoanjo merged commit 1118810 into ivoanjo:master Jul 27, 2022
casperisfine pushed a commit to Shopify/ruby that referenced this pull request Nov 9, 2023
Context: ivoanjo/gvl-tracing#4

Some hooks may want to collect data on a per thread basis.
Right now the only way to identify the concerned thread is to
use `rb_nativethread_self()` or similar, but even then because
of the thread cache or MaNy, two distinct Ruby threads may report
the same native thread id.

By passing `thread->self`, hooks can use it as a key to store
the metadata.

NB: Most hooks are executed outside the GVL, so such data collection
need to use a thread-safe data-structure, and shouldn't use the
reference in other ways from inside the hook.

They must also either pin that value or handle compaction.
byroot added a commit to ruby/ruby that referenced this pull request Nov 13, 2023
Context: ivoanjo/gvl-tracing#4

Some hooks may want to collect data on a per thread basis.
Right now the only way to identify the concerned thread is to
use `rb_nativethread_self()` or similar, but even then because
of the thread cache or MaNy, two distinct Ruby threads may report
the same native thread id.

By passing `thread->self`, hooks can use it as a key to store
the metadata.

NB: Most hooks are executed outside the GVL, so such data collection
need to use a thread-safe data-structure, and shouldn't use the
reference in other ways from inside the hook.

They must also either pin that value or handle compaction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants